fix(loop): publish trusted control plane on macOS#109
Conversation
Traviinam
left a comment
There was a problem hiding this comment.
Fresh-context COMMENT-only review
RVW-1-1-1 — P1 · High confidence — make the test independent of a host-installed gh
File: loops/issue-dev-loop/tests/install-trusted-control-plane.test.mjs:95
The new test launches the real installer with the inherited PATH. The installer requires gh, but the authoritative verifier image installs only ca-certificates and git (loops/issue-dev-loop/scripts/verifier.Dockerfile:1-5). The evidence workflow runs pnpm verify in that image (.github/workflows/issue-dev-loop-evidence.yml:148-151), and package.json:78-79 now includes this test. Consequently, future issue-loop evidence verification will fail before it can be accepted.
Reproduction at the immutable head:
env PATH=/usr/bin:/bin /Users/leyoonafr/.nvm/versions/node/v24.5.0/bin/node --test loops/issue-dev-loop/tests/install-trusted-control-plane.test.mjs
Result: required executable is unavailable during trusted installation: gh
The same test passes with the host PATH containing gh.
Expected resolution: create an executable fixture gh and prepend its directory to the installer subprocess PATH, keeping the test self-contained. Alternatively, provide gh in every canonical verifier environment and verify the containerized pnpm verify path.
Inline comment ID: null (body-only finding).
Spec
PASS. The implementation correctly defers the temporary root mode 0555 until after rename, switches cleanup ownership after successful publication, preserves read-only child and file modes, and retains canonical manifest and source binding. No scope creep found.
Residual test risk: cleanup failure branches and recursive installed modes are not directly exercised.
|
RVW-1-1-1 classification: accepted. Ethandasw reproduced and fixed the verifier portability issue in commit 8edc9751070f29d4cf6d27c4e3f616d074308bb3, published as Draft follow-up PR #110: #110 Evidence:
The test now supplies its own executable gh fixture, prepends it only for the installer subprocess, and asserts the generated manifest binds that fixture. |
Summary
Verification
Risk
Low and isolated to trusted control-plane installation. Installed files and child directories are read-only before publication; the bundle root is set to mode 0555 immediately after the atomic rename.
Bootstrap note
This Draft PR repairs the installer required to activate issue-dev-loop. After owner merge into dev, install a fresh trusted control plane from the clean owner-merged dev checkout before resuming the loop.
Owner controls Ready, approval, and merge.